Add code symbols into outline#972
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
I attempted to add tests until I remembered that our test setup does not have actual LSPs for Python or R in it, so we can't really test this. Well... now that I write that, I realize I could probably mock one out. That seems a little contrived, but maybe still worth it. |
juliasilge
left a comment
juliasilge
left a comment
There was a problem hiding this comment.
I seem to be getting two of everything, at least in Positron:
|
I was able to reproduce duplicated language symbols in code cells in Positron with inline outputs enabled. Looking into what is happening. |
|
@juliasilge In await commands.executeCommand<DocumentSymbol[] | SymbolInformation[]>(
"vscode.executeDocumentSymbolProvider",
uri
);I checked the surrounding context, including the vdoc content. Everything seems to be in order, nothing strange otherwise. When I disabled |
|
Cell Diagnostics tests were failing with a message about timing out in CI. The current run of the tests passed, but there wasn't a code change that would've affected the tests or the code tested. The tests seem to be flakey for some reason. Let's keep our eye on that. |
juliasilge
left a comment
juliasilge
left a comment
There was a problem hiding this comment.
I looked into the issue with inline output and it is some change we'll need to make over on the Positron side; I'll log that once this is merged.
I do think we need to add at least some basic tests, with a fake in-process symbol provider (not a real LSP). We already do this for the formatting tests. I think the steps would be:
- Register a temporary provider in
symbols.test.tswithvscode.languages.registerDocumentSymbolProvider({ scheme: "file", pattern: "**/.vdoc.*" }, ...)so it only handles Quarto virtual docs. - Make that provider return each shape in separate tests:
DocumentSymbol[]SymbolInformation[]undefined
- Run
vscode.executeDocumentSymbolProvideronformat/basics.qmdor another example file and assert:- no throw for
undefined - code-cell symbols still exist
- returned embedded symbol names appear under/alongside code-cell outline entries for both result shapes.
- no throw for
That will give us coverage of the middleware logic without having to set up real LSPs.
| "vscode.executeDocumentSymbolProvider", | ||
| uri | ||
| ); | ||
| if (result.length === 0) return undefined; |
There was a problem hiding this comment.
The result here could also be nothing at all (undefined probably?) so it might be nicer to handle that in this if so it doesn't show up as an error in that catch().
There was a problem hiding this comment.
How do you know it could be nothing at all? The https://code.visualstudio.com/api/references/commands seems to say that it will always resolve to a list (although it is slightly ambiguous if the type of the list is (SymbolInformation | DocumentSymbol)[] or if its SymbolInformation[] | DocumentSymbol[]). Google search summary says it resolves to SymbolInformation[] | DocumentSymbol[] but with no good citation. Do you know a way to figure out the return type of a command? I attempted to find the definition of the command in the vscode repo, but couldn't.
| if (result.length === 0) return undefined; | ||
|
|
||
| if (isDocumentSymbol(result[0])) { | ||
| return unadjustSymbolRanges(result as DocumentSymbol[], vdoc.language, cellRange.start.line); |
There was a problem hiding this comment.
Is it possible to also handle results that come back as SymbolInformation[], as indicated in line 457?
There was a problem hiding this comment.
yes, committed a change for this.
3 participants
Addresses an issue mentioned by @juliasilge: #167 (comment)
Adds code symbols under code cells in the outline.